Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trend reports are based on distribution/donation/purchase issued date #4769

Merged
merged 4 commits into from
Nov 21, 2024

Conversation

jp524
Copy link
Contributor

@jp524 jp524 commented Nov 10, 2024

Resolves #4738

Description

There was a difference between the data shown on the Distributions page and the Distributions Trend Report. The Distribution page was showing quantities per distribution's issued date, while the Trend Report page was showing quantities per line item's creation date.

The trend report uses HistorialTrendService to build the data that will be displayed. Modifying this service also fixed trend reports for donations and purchases.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Modified and added tests to HistoricalTrendService.
Also visually confirmed results by comparing trend report to filtered data (refer to screenshots below).

Screenshots

Distribution trend report from before:
Screenshot 2024-11-10 at 09 13 54

After:

Filtered data Trend report
Distributions Distributions filtered Distributions trend report
Donations Donations filtered Donations trend report
Purchases Purchases filtered Purchases trend report

@cielf cielf requested review from dorner and cielf November 12, 2024 15:48
Copy link
Collaborator

@cielf cielf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM from a functional pov. Over to @dorner for technical comments.

Copy link
Collaborator

@dorner dorner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor request, otherwise looks good.

app/services/historical_trend_service.rb Outdated Show resolved Hide resolved
@jp524 jp524 requested a review from dorner November 17, 2024 13:40
@dorner
Copy link
Collaborator

dorner commented Nov 21, 2024

Looks good, thanks!

@dorner dorner merged commit a1d71fc into rubyforgood:main Nov 21, 2024
11 checks passed
@jp524 jp524 deleted the 4738-fix-trend-reports branch November 21, 2024 19:58
Copy link
Contributor

@jp524: Your PR Trend reports are based on distribution/donation/purchase issued date is part of today's Human Essentials production release: 2024.11.24.
Thank you very much for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Investigation] Mismatch between trend report and filtered distribution
3 participants